Add ZHA migration retry steps for unplugged adapters #155537
Conversation
home-assistant
bot
added
cla-signed
Hacktoberfest
has-tests
integration: zha
new-feature
labels
|
Hey there @dmulcahey, @Adminiuga, @puddly, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
| # config entry should always exist here, skip if not | ||
| if not config_entries: | ||
| return await self.async_step_maybe_confirm_ezsp_restore() |
There was a problem hiding this comment.
This is the one line missing test coverage. Can write a test for this, but we should never be able to hit this.
We could assert that config_entries is not empty, or we could also save the "old adapter device path" to some instance variable in async_step_maybe_reset_old_radio.
Passing it as an additional argument also works, but it's not a pattern I've seen used for any config flows really.
There was a problem hiding this comment.
You conceivably could hit this by deleting ZHA halfway through the flow but I agree, this isn't something we should care about. There are many places in Core that break if you try hard enough.
There was a problem hiding this comment.
So the test added I added in bdf3a2c (#155537) won't actually help to cover this, since the plug_in_old_radio won't happen again. The retry_old_radio step goes to maybe_reset_old_radio and that step itself will skip to maybe_confirm_ezsp_restore, as there are no other config entries anymore.
For this line to be hit, the config entry needs to be removed at precisely the exact moment we're trying to reset the adapter. I'll try something different..
There was a problem hiding this comment.
bcb573c (#155537) should cover this now and work completely reliable, even if this is almost impossible for a user to do: remove both the config entry and unplug the adapter whilst we try to reset it.
There was a problem hiding this comment.
Alternative to this would be saving or passing the old adapter's path from async_step_maybe_reset_old_radio. Then, we wouldn't need to get config entries in the new step displaying the old adapter's path, so we could avoid the potential step skip if the entry is removed in time, thus avoiding the whole tests for it as well.
But now, we have the test and everything functions as expected if the user is able to hit this 😄
There was a problem hiding this comment.
Pull Request Overview
This PR adds retry logic for handling unplugged Zigbee adapters during the ZHA migration/setup flow. When an adapter connection fails, users are now prompted to plug the adapter back in and retry, rather than the flow failing outright.
Key changes:
- Added error handling for
HomeAssistantErrorexceptions during adapter reset and restore operations - Implemented two new config flow steps:
plug_in_old_radioandplug_in_new_radioto handle adapter connection failures - Added corresponding translation strings for the new user-facing prompts
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| homeassistant/components/zha/config_flow.py | Added error handling and retry steps for when adapters are unplugged during migration/setup |
| homeassistant/components/zha/strings.json | Added translation strings for new plug-in prompts in both config and options flows |
| tests/components/zha/test_config_flow.py | Added comprehensive tests for retry scenarios when adapters are unplugged |
|
Will wait on the merge as you comment on copilot review that you will fix it |
|
Per "discussion" in the last Copilot comment above (#155537 (comment)), I've also updated the string for the old adapter reset dialog to include the "warning" text. That gives it a bit more emphasis on when you want to do this (or when not). Also posted screenshots for a comparison of both dialog variants there. If we do not want this, I can just revert the last commit. |
|
Looks good to me! |
TheJulianJES
deleted the
tjj/zha_config_flow_guard_old_adapter_removal
branch
6 participants
Proposed change
This PR fixes an issue where the ZHA migration completely fails if the user unplugs the old or new radio in between steps.
New dialogs are added in the config/options flow to allow the user to plug back in the old or new adapter respectively.
Initial implementation of puddly's suggestion here: zigpy/backlog#59 (comment)
No user-facing changes with old + new adapters plugged in
If both adapters are plugged in during the entire migration (which we expected when it was rewritten), nothing changes!
These dialogs are only shown if the user unplugged the old or new adapter during the migration, when we need to communicate with it to (1) reset or (2) restore the backup.
Images of new dialogs
Previous dialog for old adapter being unplugged without the warning (click to view)
This is the initial version of the dialog, before the change introduced in this commit:
f594696(#155537)Config flow steps
When migrating adapters, something like this happens in the background:
Currently, if the old adapter is unplugged for step 4, the whole migration blows up.
The same also applies if the new adapter is unplugged for step 5.
Why do users do this?
Sometimes, users have a valid reason to unplug the old adapter at that point: they have a device with only one (free) USB port for their Zigbee adapter. So, we should not catastrophically fail anymore.
With this PR, retry steps are added to step 4 and 5:
Allowing migration with one USB port
Theoretically, this also allows the user to perform a migration with just one free USB port for the Zigbee adapter.
Mostly, you just need to follow the instructions. There are two new main possibilities for this:
Detailed(!) steps for doing migration with one USB port (CLICK TO OPEN)
'Live' migration
This reset can also be skipped (continue with step 17), otherwise proceed normally:
Old backup migration
*: It is also possible to reset the old adapter at this point. It just needs to be plugged in briefly. The new adapter can be unplugged during that time. Another dialog will ask to plug in the new adapter afterwards.
This is very similar to steps 10 to 16 from the "Live migration".
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: